Add proxy support: honor system proxy and HTTPS_PROXY/HTTP_PROXY/NO_PROXY#19
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the networking layer to better support corporate/headless proxy setups, while also hardening credential input handling and cleaning up legacy source layout.
Changes:
- Update
URLSessionsetup to honor macOS system proxy/PAC by default and addHTTPS_PROXY/HTTP_PROXY+NO_PROXYenv-var support. - Sanitize OAuth identifiers (client ID / key ID) to avoid clipboard newline/whitespace issues and validate single-line constraints in CLI/App entry points.
- Remove the unused legacy
Sources/asbmutil/implementation and document proxy behavior in the README.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/core/Utilities/StringSanitization.swift | Adds a shared string sanitization helper for identifiers. |
| Sources/core/Auth/Creds.swift | Applies identifier sanitization when loading credentials and deriving scope. |
| Sources/core/API/APIClient.swift | Removes proxy-disabling session; adds env-var proxy handling and NO_PROXY logic. |
| Sources/cli/ConfigCommand.swift | Sanitizes and validates identifier inputs before saving credentials. |
| Sources/app/Views/Settings/SettingsView.swift | Sanitizes identifiers when creating a new profile in the UI. |
| Sources/app/ViewModels/SettingsViewModel.swift | Sanitizes/validates identifiers on save with a user-facing error state. |
| README.md | Documents system proxy + env-var proxy behavior and examples. |
| Sources/asbmutil/FileCredentialStore.swift | Removes legacy file-based credential store implementation. |
| Sources/asbmutil/APIClient.swift | Removes legacy API client implementation that disabled system proxy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func bypassed(_ host: String) -> Bool { | ||
| let h = host.lowercased() | ||
| return bypass.contains { pattern in | ||
| let p = pattern.hasPrefix(".") ? String(pattern.dropFirst()) : pattern |
Comment on lines
+622
to
+625
| ```bash | ||
| export HTTPS_PROXY=http://proxy.corp.example:8080 | ||
| export NO_PROXY=*.internal.example,localhost | ||
| ``` |
Comment on lines
+156
to
160
| } | ||
|
|
||
| HStack { | ||
| Button("Cancel") { showNewProfile = false } | ||
| .keyboardShortcut(.cancelAction) |
Leftover from PR #15's move to Sources/core/. Not referenced by Package.swift or anything else in the repo, so it never compiled.
The previous code force-disabled proxies by setting connectionProxyDictionary to an empty dictionary, which made the tool unusable in environments that require an HTTP proxy for outbound traffic and ignored standard proxy env vars even when set. Drop the override so URLSession uses the macOS system proxy / PAC by default, and add explicit HTTPS_PROXY / HTTP_PROXY / NO_PROXY env-var support for headless and CI use. NO_PROXY is matched against the auth host (account.apple.com) and the scope-specific API host; if every host is bypassed, the env-var proxy is skipped and system proxy applies. Closes #16
462af34 to
28f67cc
Compare
Treat `*.foo.com` the same as `.foo.com` and `foo.com` for NO_PROXY matching, since the README and common curl/Docker NO_PROXY conventions use the `*.` form. Without this, `NO_PROXY=*.apple.com` failed to bypass the auth and API hosts as the example in the README implied.
kCFNetworkProxiesHTTPSEnable/Proxy/Port are CoreFoundation constants unavailable on swift-corelibs-foundation. On Linux, fall through to the default URLSessionConfiguration which already honors HTTP(S)_PROXY env vars via the underlying networking stack.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
connectionProxyDictionary = [:]override inAPIClientso URLSession honors the macOS system proxy / PAC by default, fixing the unusable-behind-corp-proxy case reported in FR: Add proxy support #16.HTTPS_PROXY/HTTP_PROXY/NO_PROXYenv-var support for headless and CI environments where system proxy isn't configured.Sources/asbmutil/directory left over from PR Ship ASBMUtil.app: native SwiftUI front-end + tag-only releases #15's layout move (not referenced byPackage.swiftor anything else).NO_PROXYis matched againstaccount.apple.com(auth) and the scope-specific API host (api-business.apple.comorapi-school.apple.com). If every host this client reaches is bypassed, the env-var proxy is skipped so system proxy applies. Lowercase variants (https_proxy, etc.) are honored too.Proxy authentication relies on system credentials (Negotiate/Kerberos); user:pass-in-URL embedding is not extracted in this PR — happy to add a follow-up if needed.
Closes #16.
Test plan
swift buildsucceedsasbmutil list-mdm-serversagainst a tenant on a network without a system proxy — should work as beforeHTTPS_PROXY=http://localhost:8080pointing at a local proxy (mitmproxy/Charles), run a command — traffic should flow through the proxyNO_PROXY=*.apple.com, the env-var proxy should be skipped and the request should bypass it